Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

separate header shortcuts from other accessors #1913

Merged
merged 1 commit into from
Sep 20, 2023

Conversation

karenetheridge
Copy link
Contributor

This improves readability by making it easier to find the methods that aren't
simply shortcuts to particular headers.
I have confirmed that the headers listed in %NAMES exactly matches those in
the pod, except for referer and referrer which have custom subs.

kathackeray
kathackeray previously approved these changes Feb 21, 2022
Copy link
Contributor

@kathackeray kathackeray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Confirmed all headers in %NAMES have a =head2 entry
  • Confirmed all =head2 entries not in %NAMES are not header shortcuts...
  • ...except referrer and referer, as explained in the PR description, which do not make use of generated accessors

@karenetheridge
Copy link
Contributor Author

I will happily rebase this branch (and confirm that the changes are still accurate) if I can get this reviewed -- but I lack the permissions to tag the team for review.

@kraih kraih requested review from a team, marcusramberg, kraih, jberger and Grinnz September 19, 2023 11:47
Grinnz
Grinnz previously approved these changes Sep 19, 2023
@karenetheridge
Copy link
Contributor Author

Rebased! git diff 1fc8c1c68edf6b630fc5d873fbe6ef82aeb2960f..upstream/main lib/Mojo/Headers.pm shows that there weren't many changes made in the meantime, and no conflicts were introduced.

@mergify mergify bot dismissed Grinnz’s stale review September 20, 2023 03:40

Pull request has been modified.

@mergify
Copy link
Contributor

mergify bot commented Sep 20, 2023

This pull request is now in conflicts. Could you fix it @karenetheridge? 🙏

This improves readability by making it easier to find the methods that aren't
simply shortcuts to particular headers.
I have confirmed that the headers listed in %NAMES exactly matches those in
the pod, except for referer and referrer which have custom subs.
Copy link
Member

@jhthorsen jhthorsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree 👍

@mergify mergify bot merged commit d11b23e into mojolicious:main Sep 20, 2023
10 checks passed
@karenetheridge karenetheridge deleted the ether/header-docs branch September 20, 2023 04:52
@karenetheridge
Copy link
Contributor Author

thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants